-
Notifications
You must be signed in to change notification settings - Fork 114
feat: update registry coordinator for new createOperatorSets
#544
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release-dev/ux-updates
Are you sure you want to change the base?
feat: update registry coordinator for new createOperatorSets
#544
Conversation
1fbeb57 to
b31bebd
Compare
Update readme for new table calcs deployed fix: compile fix: tests fix: ci pin version fix: pin forge test: add skips chore: fmt
b31bebd to
2b08af7
Compare
|
Note: storage layout appears to be flaky here. No difference between the two and there were no changes to storage of the |
|
|
||
| /// @notice For delegated stake quorums, the address that is set to slash | ||
| /// @dev This address is set to the burn address as 0 addresses are not valid slasher addresses | ||
| address public constant DELEGATED_STAKE_SLASHER = 0x00000000000000000000000000000000000E16E4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| address(registryCoordinator), | ||
| allocationManagerAddr, | ||
| IAllocationManager.createOperatorSets.selector | ||
| bytes4(keccak256("createOperatorSets(address,(uint32,address[],address)[])")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guessing this is because there's now two createOperatorSets aliases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup :)
| AllocationManagerView allocationManagerView = new AllocationManagerView( | ||
| delegationManager, eigenStrategy, uint32(7 days), uint32(1 days) | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Named parameters preferred for readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Constants also work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test/utils/MockAVSDeployer.sol
Outdated
| ); | ||
|
|
||
| allocationManagerView = | ||
| new AllocationManagerView(delegationMock, eigenStrategy, uint32(7 days), uint32(1 days)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Constants pls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ] | ||
| # Specifies the exact version of Solidity to use, overriding auto-detection. | ||
| solc_version = '0.8.27' | ||
| solc_version = '0.8.29' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe want to double check pragmas, especially for ALM view (must be >=0.8.29).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pragmas should be fine since they don't need to be 0.8.29, only ALM, right?
0xClandestine
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some nits, looks good

Motivation:
As part of Layr-Labs/eigenlayer-contracts#1645, we are adding a new
createOperatorSetsfunction that passes in the slasher address upon creation of an operatorSet. Although the oldcreateOperatorSetsfunction is not being deprecated, we are updating theRegistryCoordinatorin order to be only compatible the new function.Modifications:
createSlashableStakeQuorumto take in aslasheraddresssolcversion infoundry.tomlto0.8.29, matching CoreResult:
Compatible with updated
AllocationManagerinterface